Skip to content

[compiler] Add fault tolerance test fixtures#556

Closed
everettbu wants to merge 8 commits into
mainfrom
pr35835
Closed

[compiler] Add fault tolerance test fixtures#556
everettbu wants to merge 8 commits into
mainfrom
pr35835

Conversation

@everettbu

@everettbu everettbu commented Feb 20, 2026

Copy link
Copy Markdown

Mirror of facebook/react#35835
Original author: josephsavona



Stack created with Sapling. Best reviewed with ReviewStack.

  • #35838
  • #35837
  • #35836
  • -> #35835
  • #35834
  • #35833
  • #35832
  • #35831
  • #35830
  • #35829
  • #35828

Add detailed plan for making the React Compiler fault-tolerant by
accumulating errors across all passes instead of stopping at the first
error. This enables reporting multiple compilation errors at once.
Add error accumulation methods to the Environment class:
- #errors field to accumulate CompilerErrors across passes
- recordError() to record a single diagnostic (throws if Invariant)
- recordErrors() to record all diagnostics from a CompilerError
- hasErrors() to check if any errors have been recorded
- aggregateErrors() to retrieve the accumulated CompilerError
- tryRecord() to wrap callbacks and catch CompilerErrors
…erance

- Change runWithEnvironment/run/compileFn to return Result<CodegenFunction, CompilerError>
- Wrap all pipeline passes in env.tryRecord() to catch and record CompilerErrors
- Record inference pass errors via env.recordErrors() instead of throwing
- Handle codegen Result explicitly, returning Err on failure
- Add final error check: return Err(env.aggregateErrors()) if any errors accumulated
- Update tryCompileFunction and retryCompileFunction in Program.ts to handle Result
- Keep lint-only passes using env.logErrors() (non-blocking)
- Update 52 test fixture expectations that now report additional errors

This is the core integration that enables fault tolerance: errors are caught,
recorded, and the pipeline continues to discover more errors.
…rs on env

Update 9 validation passes to record errors directly on fn.env instead of
returning Result<void, CompilerError>:
- validateHooksUsage
- validateNoCapitalizedCalls (also changed throwInvalidReact to recordError)
- validateUseMemo
- dropManualMemoization
- validateNoRefAccessInRender
- validateNoSetStateInRender
- validateNoImpureFunctionsInRender
- validateNoFreezingKnownMutableFunctions
- validateExhaustiveDependencies

Each pass now calls fn.env.recordErrors() instead of returning errors.asResult().
Pipeline.ts call sites updated to remove tryRecord() wrappers and .unwrap().
… tolerance

Update remaining validation passes to record errors on env:
- validateMemoizedEffectDependencies
- validatePreservedManualMemoization
- validateSourceLocations (added env parameter)
- validateContextVariableLValues (changed throwTodo to recordError)
- validateLocalsNotReassignedAfterRender (changed throw to recordError)
- validateNoDerivedComputationsInEffects (changed throw to recordError)

Update inference passes:
- inferMutationAliasingEffects: return void, errors on env
- inferMutationAliasingRanges: return Array<AliasingEffect> directly, errors on env

Update codegen:
- codegenFunction: return CodegenFunction directly, errors on env
- codegenReactiveFunction: same pattern

Update Pipeline.ts to call all passes directly without tryRecord/unwrap.
Also update AnalyseFunctions.ts which called inferMutationAliasingRanges.
Add test fixture demonstrating fault tolerance: the compiler now reports
both a mutation error and a ref access error in the same function, where
previously only one would be reported before bailing out.

Update plan doc to mark all phases as complete.
@everettbu everettbu added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 20, 2026
@greptile-apps

greptile-apps Bot commented Feb 20, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds fault tolerance to the React Compiler, enabling it to continue through compilation passes and report all errors rather than stopping at the first one. The key architectural change is that the Environment class now accumulates errors via recordError/tryRecord methods, and the pipeline returns Result<CodegenFunction, CompilerError> with aggregated errors at the end.

  • Adds error accumulation infrastructure to Environment (#errors, recordError, recordErrors, tryRecord, hasErrors, aggregateErrors) with invariants still re-thrown immediately
  • Changes lower() to always produce an HIRFunction (even partial), treating unsupported constructs like var as let and handling for(;;) with placeholder expressions
  • Wraps all pipeline passes in env.tryRecord() as a safety net, catching non-invariant CompilerError exceptions so compilation continues
  • Migrates ~15 validation passes from returning Result or throwing to recording errors on env
  • Updates codegenFunction and inference passes to record errors on env instead of returning Result
  • Adds dedicated fault-tolerance test fixtures (fault-tolerance/) that verify multiple independent errors are reported in a single compilation
  • Updates 60+ existing test fixture expectations where additional previously-masked errors are now surfaced

Confidence Score: 4/5

  • This is a well-designed architectural refactor with comprehensive test coverage and a thorough design document; the changes are internal to the compiler and do not affect runtime output for correctly-compiled code.
  • The PR follows a systematic, phased approach documented in the overview. All validation and inference passes are consistently migrated. Invariant errors still propagate immediately for safety. The test fixtures demonstrate the fault tolerance working as intended. The main risk area is the definite assignment assertion pattern for value-producing passes (e.g., let reactiveFunction!: ReactiveFunction) which could lead to runtime TypeErrors if those passes throw, though the outer try/catch in Program.ts provides a safety net.
  • Pipeline.ts (definite assignment assertions for value-producing passes) and BuildHIR.ts (partial HIR generation paths) are the most complex changes

Important Files Changed

Filename Overview
compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts Core infrastructure: adds #errors accumulator, recordError/recordErrors/tryRecord/hasErrors/aggregateErrors methods. Invariant errors are correctly re-thrown immediately. Well-structured foundation for fault tolerance.
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts Wraps all pipeline passes in env.tryRecord(), changes return type to Result<CodegenFunction, CompilerError>. Uses definite assignment assertion (!) for reactiveFunction which could be undefined if buildReactiveFunction throws. Value-producing passes use safe defaults. Final error aggregation at end of pipeline.
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts Updates tryCompileFunction and retryCompileFunction to handle Result return type from compileFn. Error handling paths correctly preserve existing behavior for retry logic.
compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts Changes lower() to always return HIRFunction. Handles var as let, fixes ForStatement init/test handling for partial HIR, adds try/catch around body lowering. Contains a redundant ternary in the catch handler. Removes effects field (not part of HIRFunction type).
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts Changes codegenFunction and codegenReactiveFunction to return directly instead of via Result. Records codegen errors on env. Outlined function codegen also updated.
compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts Changes return type to always produce Array<AliasingEffect>. Errors only recorded when enableValidations is true (matching old pipeline behavior where errors were only thrown inside enableValidations guard).
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts Refactored to pass errors from the outer function into the implementation. Inner function expressions use a separate innerErrors to avoid contaminating the outer analysis. Fixed-point loop behavior preserved.

Last reviewed commit: f773cbc

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

92 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +220 to +224
if (
(detail instanceof CompilerDiagnostic
? detail.category
: detail.category) === ErrorCategory.Invariant
) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant ternary expression
The ternary (detail instanceof CompilerDiagnostic ? detail.category : detail.category) evaluates to detail.category in both branches. Both CompilerDiagnostic and CompilerErrorDetail expose category as a direct property, so the instanceof check has no effect.

Suggested change
if (
(detail instanceof CompilerDiagnostic
? detail.category
: detail.category) === ErrorCategory.Invariant
) {
if (detail.category === ErrorCategory.Invariant) {
Prompt To Fix With AI
This is a comment left during a code review.
Path: compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
Line: 220-224

Comment:
**Redundant ternary expression**
The ternary `(detail instanceof CompilerDiagnostic ? detail.category : detail.category)` evaluates to `detail.category` in both branches. Both `CompilerDiagnostic` and `CompilerErrorDetail` expose `category` as a direct property, so the `instanceof` check has no effect.

```suggestion
          if (detail.category === ErrorCategory.Invariant) {
```

How can I resolve this? If you propose a fix, please make it concise.

@everettbu

Copy link
Copy Markdown
Author

Upstream PR was closed or merged. Code is synced via branch mirror.

@everettbu everettbu closed this Feb 21, 2026
@everettbu
everettbu deleted the pr35835 branch February 21, 2026 04:00
@everettbu
everettbu restored the pr35835 branch February 21, 2026 04:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants